Only clear ASCollectionView's data during deallocation #1154
+6
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a follow up on TextureGroup#1136. Our experiment results show that clearing data frequently is the cause of our TextureGroup#1 crash. @maicki and I believe that this is because if the collection view is being used, silently clearing its data without notifying the backing UICollectionView can put it out-of-sync and causes mayhem next time the collection view processes a batch update. If you look at the stack trace closely, you'll notice that the crash doesn't occur on the same run loop that clearData is called. This made it extremely tricky to investigate and identify the root cause. Another interesting question would be whether or not we want to clear the data during deallocation at all, since the data will be cleared out soon anyway.
nguyenhuy
requested review from
maicki,
garrettmoon,
appleguy and
Adlai-Holler
|
Will us further looking into fixing: #1155 |
mikezucc
pushed a commit
to mikezucc/Texture
that referenced
this pull request
Nov 7, 2018
…1154) This is a follow up on TextureGroup#1136. Our experiment results show that clearing data frequently is the cause of our #1 crash. @maicki and I believe that this is because if the collection view is being used, silently clearing its data without notifying the backing UICollectionView can put it out-of-sync and causes mayhem next time the collection view processes a batch update. If you look at the stack trace closely, you'll notice that the crash doesn't occur on the same run loop that clearData is called. This made it extremely tricky to investigate and identify the root cause. Another interesting question would be whether or not we want to clear the data during deallocation at all, since the data will be cleared out soon anyway.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow up on #1136. Our experiment results show that clearing data frequently is the cause of our #1 crash. @maicki and I believe that this is because silently clearing its data while the collection view is still being used, without notifying the backing UICollectionView, puts it out-of-sync and causes mayhem next time the collection view processes a batch update. If you look at the stack trace closely, you'll notice that the crash doesn't occur on the same run loop that
clearDatawas called, making it extremely tricky to investigate and identify the root cause.To fix this problem, I'm proposing to only call
clearDataif the collection view is actually being torn down. I'm also going to set up another experiment to slowly roll this out to make sure we don't reintroduce the crash, as well as to get insights on whether it's actually worth it. My gut feeling is that it's not, since the data will be autimatically cleared out soon anyway. Please let me know your thoughts.